Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

Agentic 검색 관련해서 흥미롭고 유용한 Cookbook을 만들어주셔서 감사해요 😊
관련해서 간단한 리뷰를 남겼습니다 :) 감사합니다!

전체적으로 잘 읽히는 문서네요 ㅎㅎ. 몇 군데 보면서 떠오른 것들 적어보아요!

제목 및 서론

Line 1

- 🤖 벡터 검색 에이전트: 허깅페이스 허브를 백엔드로 하는 똑똑한 검색 엔진
+ 🤖 벡터 검색 에이전트: 허깅페이스 허브 기반 지능형 검색 엔진

=> “똑똑한”은 다소 구어체 느낌이라, “지능형”으로 바꾸면 기술 문서 톤에 더 잘어울릴 거 같아요!!

그리고 그밖의 생각난 것들!!

서론

- 검색엔진은 크게 키워드 검색과 벡터 검색으로 분류 됩니다.
+ 검색 엔진은 크게 키워드 검색과 벡터 검색으로 나눌 수 있습니다.

- 키워드 검색과는 달리, 벡터 검색으로 진행할 경우 두가지를 고려해야합니다.
+ 키워드 검색과 달리, 벡터 검색을 사용할 때는 두 가지를 고려해야 합니다.

- 그러나, 임베딩값을 기반으로하는 벡터 검색만으로는 '사용자가 원하는 답변'를 보장하기 어렵습니다.
+ 하지만 임베딩 값을 기반으로 한 벡터 검색만으로는 ‘사용자가 원하는 답변’을 보장하기 어렵습니다.

- 따라서 검색의 각 단계에서 에이전트가 각 단계를 자율적으로 판단하고 최적화한다면 사용자가 원하는 답변에 가까운 검색 결과를 얻을 수 있을 것 같습니다!
+ 그래서 검색 과정에서 에이전트가 자율적으로 판단하고 최적화한다면, 사용자 의도에 더 가까운 검색 결과를 얻을 수 있습니다.

워크플로우 섹션

- 기존 벡터검색 워크플로우
+ 기존 벡터 검색 워크플로우

DuckDB 섹션

- 허깅페이스의 데이터셋은 파켓(parquet) 파일에 의존하는데 빠른 인메모리 데이터베이스 시스템인
+ 허깅페이스의 데이터셋은 파켓(parquet) 파일에 의존하는데, 빠른 인메모리 데이터베이스 시스템인

=> 쉼표 추가하면 좋지 않을까?! 하는 생각이!

섹션 제목들

- # 도구정의
+ # 도구 정의

- ### 도구1 : 임베딩 생성
+ ### 도구1: 임베딩 생성

- ### 도구2 : DuckDB인덱스 만들기
+ ### 도구2: DuckDB 인덱스 만들기

- ### 도구4 :답변 생성 도구
+ ### 도구4: 답변 생성 도구

Docstring

Args:
-   column_name: 임베딩할 컬럼 이름
+   column_name: 임베딩할 열 이름

=> “컬럼” 대신 “열”로 통일하는 것이 좋지 않을까요!

기타

- 무거운 벡터 검색 엔진을 배포할 필요가 없고 저장소는 허브에서 처리됩니다.
+ 무거운 벡터 검색 엔진을 따로 배포할 필요 없고, 저장소는 허브에서 처리됩니다.

수고 많으셨습니다! 안정님께서 활약해주신 덕분에 프로젝트가 더 잘되는거 같아요 ㅋㅋ

정님🤗 굉장히 흥미로운 주제의 notebook 이네요!

띄어쓰기와 같은 약간의 수정만 해봤는데,
정님께서 한번 검토하시고 반영해주시면 감사하겠습니다!

제작하시느라 고생 많으셨습니다👏🏻

우와! DuckDB를 활용한 예제라니~! 👍 정말 수고 많으셨습니다 😊 정님 글은 늘 자연스럽게 읽혀서 좋은글이라고 느껴집니다. 이번 글도 DuckDB를 활용한 벡터 검색 엔진 교보재로 손색이 없다는 생각이 드네요 ㅎㅎ 직접 작성하신 글이라 그런지 더 귀한 것 같아요.

TaskerJung님과 HyunZ118님께서 이미 꼼꼼히 리뷰를 해주셔서, 저는 직접 ipynb 파일을 다운로드해 실행해보며 검토해봤습니다.

리뷰 내용이 많아 보일 수 있지만, 사실은 반복되는 표현들이 있어서 그렇게 많지는 않습니다(?) 😄

제안 드릴 내용을 4가지로 압축할 수 있을 것 같아요!

1. 데이터셋 -> 데이터 세트

• 저도 데이터셋 어감이 더 자연스럽기는 하지만 저기 glossary기준으로 데이터 세트로 용어를 통일하고 있어서 제안해봤습니다. 번역 글은 아니기에 꼭 통일할 필요가 없다고 생각하지만, 그래도 전체적인 Hugging Face 문서의 통일성을 가져가보면 어떨까해서 제안해봐요

2. 라이브러리 다운로드
   다른 번역된 Cookbook을 보니 라이브러리 사용전 Cookbook을 실행해보는 사용자들을 위해서 초기에 라이브러리 설치하는 셀을 따로 마련해두더라구요. 라이브러리 독자들에게 pip로 다운로드를 할 수 있게 하면 어떨까 해서 제안해봅니다. 그리고 실행해보니 python 3.10버전 이상으로 업데이트해야 문제 없이 동작시킬 수 있더라구요. 그 부분에 대한 안내도 추가되면 좋을 것 같습니다.

# 본 예제 파일은 Python 3.10 이상 버전에서만 실행할 수 있습니다.이전 버전을 사용 중이시라면, 원활한 실행을 위해 Python을 최신 버전으로 업데이트하시는 것을 권장드립니다.

%pip install -U smolagents datasets sentence-transformers duckdb openai

3. import 중복
   처음에 실습에 필요한 라이브러리들을 설치하는 셀을 잘 만들어주셨는데요,
   중간에 함수 사이에 import 문이 끼어 있거나, 이미 불러온 라이브러리를 다시 불러오는 부분이 있었습니다.
   초기에 선언을 잘 해주셨으니, 해당 부분은 정리해보셔도 좋을 것 같아요.
   물론 의도하신 부분이라면 그대로 두시고 resolved로 남겨주셔도 괜찮습니다 😊
4. docstring이 없는 함수
   거의 모든 함수에 docstring을 꼼꼼히 작성해주셔서 실습의 흐름을 따라가기 수월했습니다.
   다만 일부 함수에서는 docstring이 누락되어 있었어요. 함수가 충분히 단순해서 의도하신 부분이라면 그대로 두시고 resolved로 처리해주셔도 괜찮습니다.
   그렇지 않다면 docstring을 추가해주시면 더 좋을 것 같아요 😊
5. key와 관련된 안내
   현재 ipynb 파일을 실행하기 위해서는 두 가지 키 등록이 필요해보여요.
   Ipynb파일을 그대로 실행해보니 사용자는 관련 에러를 해결해야하더라구요.
   첫 번째는 OpenAI API Key, 두 번째는 hf auth login 키입니다.

OpenAI API Key, hf auth login키를 등록하시는 방법은 잘 아시겠지만, 바로 수정하실 수 있도록 짧은 코드 스니펫을 아래 제공합니다.

import os
os.environ["OPENAI_API_KEY"] = "YOUR KEY"
os.environ["HF_TOKEN"] = "YOUR HF KEY"

만약 Cookbook에서 키 등록 방법을 따로 안내하지 않는 것이 일반적인 관례라면, 그대로 resolved로 두셔도 좋을 것 같습니다. 다만, 상단에 “두 가지 키를 미리 발급받아야 한다”는 안내를 간단히 추가해주신다면, 사용자가 정님이 작성하신 Cookbook을 처음부터 끝까지 에러 없이 한 번에 실행해볼 수 있을 것 같아 이렇게 제안드려봅니다 😊

주원님! 꼼꼼한 리뷰 감사드립니다.
쿡북 제작 가이드에 설치나, 키 관련 부분은 제외해달라는 부분이 있어서 그 부분을 제외하였는데, 다시 한번 재확인한 후 말씀주신 부분 적용해보겠습니다 :)
import 중복, docstring 누락은 확인해서 지원하도록 하겠습니다 :D

```diff
무거운 벡터 검색 엔진을 따로 배포할 필요 없고, 저장소는 허브에서 처리됩니다.

현상님! (게으름이슈로) 좋은 리뷰를 이제 발견했네요...제목은 정말 고민하던 부분이었는데 반영하였습니다!! 감사합니다 :)

Agentic 검색 관련해서 흥미롭고 유용한 Cookbook을 만들어주셔서 감사해요 😊 관련해서 간단한 리뷰를 남겼습니다 :) 감사합니다!

용우님, 좋은 리뷰 감사드립니다 :)

정님🤗 굉장히 흥미로운 주제의 notebook 이네요!

띄어쓰기와 같은 약간의 수정만 해봤는데, 정님께서 한번 검토하시고 반영해주시면 감사하겠습니다!

제작하시느라 고생 많으셨습니다👏🏻

현지님 좋은 리뷰 감사드려요 :)

빠르게 Merge 하기 전 한 부분만 확인하면 좋을 것 같아 문의드립니다!

1. Pythonic Code
   보니까 함수 안쪽에서 import가 이루어지고 있네요.
   이건 큰 문제는 아니지만, 혹시 의도하신 게 아니라면 한 번만 확인 부탁드리려 합니다 🙏

함수 내부에 import가 들어가면 함수가 호출될 때마다 import 구문이 실행되면서 약간의 오버헤드가 생길 수 있고, PEP8 기준에서도 보통은 import를 파일 상단으로 모으는 쪽을 권장하더라고요.
(물론 특정 케이스에서 lazy loading 의도라면, 그대로 두는 것도 충분히 합리하다고 생각합니다!)

수정 후 제안 :

# 요렇게 앞으로 빼면 어떨까요?!
from sentence_transformers import SentenceTransformer
import duckdb

@tool
def similarity_search_with_duckdb_index(
    query: str,
    table_name: str,
    model_id: str,
    k: int = 5,
    embedding_column: str = "embeddings"
)-> dict:
    """
    DuckDB 인덱스를 이용해 벡터 검색을 수행합니다.

    Args:
        query: 검색할 쿼리 문자열
        model_id: 임베딩에 사용할 모델
        k: 반환할 결과 수
        table_name: 검색할 테이블 이름
        embedding_column: 임베딩 컬럼 이름

    Returns:
        dict: 검색 결과
    """
    
    model = SentenceTransformer(model_id)
    embedding = model.encode(query).tolist()
    return duckdb.sql(
        query=f"""
        SELECT *, array_cosine_distance({embedding_column}_float, {embedding}::FLOAT[{model.get_sentence_embedding_dimension()}]) as distance
        FROM {table_name}
        ORDER BY distance
        LIMIT {k};
    """
    ).to_df()

2. 추가확인
   코드 보면서 한 가지 궁금했던 부분이 내부에 또 다른 def가 정의된 inner function으로 구현되어 있더라구요

Python에서 함수 안에 함수를 정의하는 건 가능하긴 하지만 보통은 클로저나 스코프를 한정하려는 등 특별한 목적이 있을 때 사용하는 경우가 많은 것으로 알고 있어서요. 의도하신 부분이 아니라면, embed_batch를 바깥으로 분리해두는 것도 고려해볼 만할 것 같습니다 🙂

또, 제가 앞서 말씀드렸던 docstring 부분도 inner function이 아니라 외부 함수로 분리되면 더 명확하게 정리할 수 있을 것 같아요.
이렇게 하면 사용자 입장에서도 함수의 역할을 바로 파악할 수 있어서 이해도가 높아질 것 같습니다.

다만 이 함수가 model이나 column_name 같은 바깥 변수를 참조하고 있어서, 그 부분은 살짝 구조를 수정해줘야 할 것 같아서 그래서 그 점을 반영한 형태로 제안드려봅니다.

수정후

from sentence_transformers import SentenceTransformer

def embed_batch(batch, model, column_name):
    """
    주어진 배치에 대해 임베딩을 생성합니다.

    Args:
        batch: 입력 데이터 배치
        model: SentenceTransformer 모델 인스턴스
        column_name: 임베딩할 컬럼 이름

    Returns:
        임베딩이 추가된 배치
    """
    embeddings = model.encode(batch[column_name], convert_to_numpy=True)
    batch["embeddings"] = embeddings.tolist()
    return batch

@tool
def create_embeddings(
    dataset: Dataset,
    model_id: str,
    column_name: str,
    ) -> Dataset:
  """
    주어진 데이터셋에 대해 임베딩을 생성합니다.

    Args:
        dataset: 임베딩을 생성할 대상 데이터셋
        model_id: 임베딩에 사용할 모델
        column_name: 임베딩할 열 이름

    Returns:
        임베딩이 추가된 데이터셋
  """
    model = SentenceTransformer(model_id)
    dataset = dataset.map(lambda batch: embed_batch(batch, model, column_name), batched=True)
    return dataset

리뷰에 남겨주신 코멘트나 디스코드에서 말씀해주시는 부분들을 보면 항상 꼼꼼하게 작업하시는 것 같다는 생각이들어요.
(특히 자연스러운 어투를 신경쓰시면서 번역하시는 것에 많이 배우고 있어요.)

저도 답변 주신 내용에서 배울 부분이 많다고 생각해서 제 의견도 함께 남겨봅니다 🙂
이 역시도 마찬가지로 의도한 부분이었다면 추가 수정 없이 Approve 요청 주시면 바로 approve해드릴게요!

추가로, 제가 최종 보고서를 작성하면서 GitHub API로 PR 내역을 확인했는데요.
이번에 정말 cookbook, model card, 번역, 그리고 저희 프로젝트까지
다양한 역할을 모두 담당하셨더라구요!
정말 고생 많으셨습니다 🙌

빠르게 Merge 하기 전 한 부분만 확인하면 좋을 것 같아 문의드립니다!

1. Pythonic Code
   보니까 함수 안쪽에서 import가 이루어지고 있네요.
   이건 큰 문제는 아니지만, 혹시 의도하신 게 아니라면 한 번만 확인 부탁드리려 합니다 🙏

함수 내부에 import가 들어가면 함수가 호출될 때마다 import 구문이 실행되면서 약간의 오버헤드가 생길 수 있고, PEP8 기준에서도 보통은 import를 파일 상단으로 모으는 쪽을 권장하더라고요. (물론 특정 케이스에서 lazy loading 의도라면, 그대로 두는 것도 충분히 합리하다고 생각합니다!)

수정 후 제안 :

# 요렇게 앞으로 빼면 어떨까요?!
from sentence_transformers import SentenceTransformer
import duckdb

@tool
def similarity_search_with_duckdb_index(
    query: str,
    table_name: str,
    model_id: str,
    k: int = 5,
    embedding_column: str = "embeddings"
)-> dict:
    """
    DuckDB 인덱스를 이용해 벡터 검색을 수행합니다.

    Args:
        query: 검색할 쿼리 문자열
        model_id: 임베딩에 사용할 모델
        k: 반환할 결과 수
        table_name: 검색할 테이블 이름
        embedding_column: 임베딩 컬럼 이름

    Returns:
        dict: 검색 결과
    """
    
    model = SentenceTransformer(model_id)
    embedding = model.encode(query).tolist()
    return duckdb.sql(
        query=f"""
        SELECT *, array_cosine_distance({embedding_column}_float, {embedding}::FLOAT[{model.get_sentence_embedding_dimension()}]) as distance
        FROM {table_name}
        ORDER BY distance
        LIMIT {k};
    """
    ).to_df()

2. 추가확인
   코드 보면서 한 가지 궁금했던 부분이 내부에 또 다른 def가 정의된 inner function으로 구현되어 있더라구요

Python에서 함수 안에 함수를 정의하는 건 가능하긴 하지만 보통은 클로저나 스코프를 한정하려는 등 특별한 목적이 있을 때 사용하는 경우가 많은 것으로 알고 있어서요. 의도하신 부분이 아니라면, embed_batch를 바깥으로 분리해두는 것도 고려해볼 만할 것 같습니다 🙂

또, 제가 앞서 말씀드렸던 docstring 부분도 inner function이 아니라 외부 함수로 분리되면 더 명확하게 정리할 수 있을 것 같아요. 이렇게 하면 사용자 입장에서도 함수의 역할을 바로 파악할 수 있어서 이해도가 높아질 것 같습니다.

다만 이 함수가 model이나 column_name 같은 바깥 변수를 참조하고 있어서, 그 부분은 살짝 구조를 수정해줘야 할 것 같아서 그래서 그 점을 반영한 형태로 제안드려봅니다.

수정후

from sentence_transformers import SentenceTransformer

def embed_batch(batch, model, column_name):
    """
    주어진 배치에 대해 임베딩을 생성합니다.

    Args:
        batch: 입력 데이터 배치
        model: SentenceTransformer 모델 인스턴스
        column_name: 임베딩할 컬럼 이름

    Returns:
        임베딩이 추가된 배치
    """
    embeddings = model.encode(batch[column_name], convert_to_numpy=True)
    batch["embeddings"] = embeddings.tolist()
    return batch

@tool
def create_embeddings(
    dataset: Dataset,
    model_id: str,
    column_name: str,
    ) -> Dataset:
  """
    주어진 데이터셋에 대해 임베딩을 생성합니다.

    Args:
        dataset: 임베딩을 생성할 대상 데이터셋
        model_id: 임베딩에 사용할 모델
        column_name: 임베딩할 열 이름

    Returns:
        임베딩이 추가된 데이터셋
  """
    model = SentenceTransformer(model_id)
    dataset = dataset.map(lambda batch: embed_batch(batch, model, column_name), batched=True)
    return dataset

리뷰에 남겨주신 코멘트나 디스코드에서 말씀해주시는 부분들을 보면 항상 꼼꼼하게 작업하시는 것 같다는 생각이들어요. (특히 자연스러운 어투를 신경쓰시면서 번역하시는 것에 많이 배우고 있어요.)

저도 답변 주신 내용에서 배울 부분이 많다고 생각해서 제 의견도 함께 남겨봅니다 🙂 이 역시도 마찬가지로 의도한 부분이었다면 추가 수정 없이 Approve 요청 주시면 바로 approve해드릴게요!

추가로, 제가 최종 보고서를 작성하면서 GitHub API로 PR 내역을 확인했는데요. 이번에 정말 cookbook, model card, 번역, 그리고 저희 프로젝트까지 다양한 역할을 모두 담당하셨더라구요! 정말 고생 많으셨습니다 🙌

앗 주원님 매번 꼼꼼한 리뷰 감사합니다! ☺️😎

• smolagents의 경우, tool함수가 독립적으로 동작해서 보통 함수 안에 import 를하는 구조라, 오히려 위에 있는 global import 문을 삭제했습니다아 감사해요!
• 같은 이유에서 2번째 내부 함수도 tool안에서 호출 가능하도록 독립적으로 쓰기위해 그렇게 만들었답니다! 더 좋은 agent툴 구현방식이 있다면 알고싶은데요....저도 아는지식 한에서 만들다보니 정답인지는 모르겠다는게 함정...!
  -데이터셋 -> 데이터세트 문제는, 진짜 고민했는데...! 다른 cookbook에서 그렇게 작성한것도 있고, agent에게 던지는 내용이기도 해서 그대로 결정했습니다ㅠㅠ

더 체크해주실 부분 말씀주시면 감사하구,
Approved해주시면 메인테이너 멘션해서 머지 고고해보겠습니다! 감사합니다 :)